Skip to content

Conversation

@itsmeichigo
Copy link
Contributor

@itsmeichigo itsmeichigo commented Oct 1, 2025

Part of WOOMOB-1392

Description

This PR updates the view of the booking list to support fetching bookings separately for Today, Upcoming, and All tabs. The idea is to add a container view to keep 3 lists with separate view models, and only show/hide them upon tab selection.

The logic for fetching bookings of each tab will be handled in a subsequent PR.

Testing steps

  • Log in to a CIAB store with Bookings plugin v2.
  • Select the Bookings tab.
  • Switch between the different tabs.
  • Confirm that all bookings are displayed in the All tab, while Today and Upcoming is empty.

Testing information

Tested with simulator iPhone 17 iOS 26.

Screenshots

Simulator.Screen.Recording.-.iPhone.17.-.2025-10-01.at.15.23.05.mov

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@itsmeichigo itsmeichigo added this to the 23.4 milestone Oct 1, 2025
@itsmeichigo itsmeichigo added the type: task An internally driven task. label Oct 1, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Oct 1, 2025

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 23.4. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 1, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16188-a95afcc
Version23.3
Bundle IDcom.automattic.alpha.woocommerce
Commita95afcc
Installation URL4e1c4h7kkgme0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@itsmeichigo itsmeichigo marked this pull request as ready for review October 1, 2025 09:41
Copy link
Contributor

@RafaelKayumov RafaelKayumov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as described. Left one question. Pre-approving.

Comment on lines 13 to 20
ZStack {
ForEach(BookingListTab.allCases, id: \.rawValue) { tab in
BookingListView(viewModel: viewModel.listViewModel(for: tab)) {
headerView
}
.opacity(viewModel.selectedTab == tab ? 1 : 0)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please describe the reasoning behind this approach of

  • injecting a header (tabs + filter bar) to each list view. When switching tabs, the header also has fade-in animation.
  • switching lists by toggling the opacity.

How about keeping the header above switchable list views and using a TabView:

VStack {
    headerView
    TabView(selection: $viewModel.selectedTab) {
        ForEach(BookingListTab.allCases, id: \.rawValue) { tab in
            BookingListView(viewModel: viewModel.listViewModel(for: tab)) {
            }
            .tag(tab)
        }
    }
    .tabViewStyle(.page(indexDisplayMode: .never))
}

We have other places in the app where the TabView is used.

Copy link
Contributor Author

@itsmeichigo itsmeichigo Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

injecting a header (tabs + filter bar) to each list view. When switching tabs, the header also has fade-in animation.

Hey - this was a workaround for the refresh control in the booking list. When the list is added to a navigation view with large title, pulling down the list will also pull down the big title while the header (tab bar and filter bar) stays fixed at the top, which looks bad. Injecting the header to a section inside the scroll view and ask the LazyVStack to pin the section header helped pulling the header down as well.

switching lists by toggling the opacity.

I wanted to keep the states of the lists instead of re-rendering and loading bookings again upon switching tabs.

How about keeping the header above switchable list views and using a TabView

Thanks for the great idea! I didn't think about using TabView just for the content, and TIL about hiding the index view. Using the TabView fixes both the problems above for me: managing the tab views and keeping the refresh control under the header view, instead of above the large navigation title:

Simulator Screenshot - iPhone 17 - 2025-10-02 at 14 21 24

I updated the BookingListView to remove the now redundant workarounds for the header and refresh control. I also added a check to avoid loading the booking list again if it's not empty.

Please take another look when you can 🙏

Copy link
Contributor

@RafaelKayumov RafaelKayumov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I just noticed a small gap between sort/filter bar and list. Feel free to merge when resolved.

Снимок экрана 2025-10-02 в 12 56 45 Снимок экрана 2025-10-02 в 12 57 25

@itsmeichigo
Copy link
Contributor Author

I removed the redundant spacing in a95afcc. The view should looks good now.

Untitled

@itsmeichigo itsmeichigo enabled auto-merge October 2, 2025 10:20
@itsmeichigo itsmeichigo merged commit e830619 into trunk Oct 2, 2025
13 checks passed
@itsmeichigo itsmeichigo deleted the woomob-1392-booking-list-tabs branch October 2, 2025 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants